Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use C++20 for programs that generate the HTML lists #445

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jwakely
Copy link
Member

@jwakely jwakely commented Nov 4, 2024

This currently also includes the unmerged commit from #444, because it modifies a line of code added by that commit. That should disappear from this PR once the other PR is merged.

As of version 19.1.0, Clang's libc++ doesn't provide chrono::clock_cast or chrono::parse so maybe it's too soon to make this change.

@jwakely jwakely marked this pull request as draft November 4, 2024 13:01
@jwakely jwakely force-pushed the c++20 branch 9 times, most recently from e841fea to 26cc0b0 Compare November 4, 2024 14:32
@jwakely
Copy link
Member Author

jwakely commented Nov 4, 2024

As of version 19.1.0, Clang's libc++ doesn't provide chrono::clock_cast or chrono::parse so maybe it's too soon to make this change.

And the ubuntu-latest runner-image for GitHub Actions is ubuntu-22.04 which only has clang-15, which isn't good enough. There is an ubuntu-24.04 image with a newer clang, but that image is still in beta.

And the GCC version in ubuntu-latest is insufficient to build the C++20 code too.

@jwakely jwakely force-pushed the c++20 branch 2 times, most recently from 560b76a to 7872e53 Compare November 4, 2024 14:41

auto parse_date(std::istream & temp) -> gregorian::date {
auto parse_date(std::istream & temp) -> std::chrono::year_month_day {
#if __cpp_lib_chrono >= 201803L
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chrono::parse requires GCC 14, but there's a workaround.

else {
auto mtime = fs::last_write_time(filename);
#if __cpp_lib_chrono >= 201803L
t = clock_cast<system_clock>(mtime);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chrono::clock_cast isn't supported by libc++, but there's a workaround.

@jwakely
Copy link
Member Author

jwakely commented Nov 4, 2024

There are several places in the code that should be converted to use views::filter

@jwakely jwakely force-pushed the c++20 branch 2 times, most recently from f917514 to a80f36e Compare November 4, 2024 20:04
@jwakely
Copy link
Member Author

jwakely commented Nov 4, 2024

There are several places in the code that should be converted to use views::filter

e.g.

   // This construction calls out for filter-iterators
//   std::multiset<lwg::issue, order_by_first_tag> pending_issues;
   std::vector<lwg::issue> pending_issues;
   for (auto const & elem : issues ) {
      if (predicate(elem)) {
         pending_issues.emplace_back(elem);
      }
   }

   sort(begin(pending_issues), end(pending_issues), order_by_section{section_db});

But I'm not sure if would actually be faster/simpler/better to insert into a multiset from a filter_view than the current code.

how-to-docs.html Outdated Show resolved Hide resolved
src/report_generator.cpp Outdated Show resolved Hide resolved
src/report_generator.cpp Outdated Show resolved Hide resolved
src/report_generator.cpp Outdated Show resolved Hide resolved
src/report_generator.cpp Outdated Show resolved Hide resolved
src/report_generator.cpp Outdated Show resolved Hide resolved
src/section_data.cpp Show resolved Hide resolved
src/sections.h Outdated Show resolved Hide resolved
@jwakely
Copy link
Member Author

jwakely commented Nov 4, 2024

There are several places in the code that should be converted to use views::filter

e.g.

   // This construction calls out for filter-iterators
//   std::multiset<lwg::issue, order_by_first_tag> pending_issues;
   std::vector<lwg::issue> pending_issues;
   for (auto const & elem : issues ) {
      if (predicate(elem)) {
         pending_issues.emplace_back(elem);
      }
   }

   sort(begin(pending_issues), end(pending_issues), order_by_section{section_db});

But I'm not sure if would actually be faster/simpler/better to insert into a multiset from a filter_view than the current code.

Maybe for C++23 one day, just because everything above can be replaced by:

auto pending_issues = issues | std::views::filter(predicate) | std::ranges::to<std::multiset<lwg::issue, order_by_section>>(order_by_section{section_db});

Instead of sorting several times in a row using different orderings, we
can define a single projection that defines the desired final order.
@Dani-Hub
Copy link
Member

Dani-Hub commented Nov 21, 2024

Some weeks ago I considered a similar change locally (Only switching to C++20 without anything else) to see how mingw can handle these changes. I now tested your branch successfully on my system and it seems to work well. I appreciate this course of action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants